Code Review

Overview

This PR fixes a regression where draggable keyboard-accessible elements (like motion.button) with the drag prop couldn't be dragged. The fix correctly distinguishes between clicking on the draggable element itself vs. clicking on keyboard-accessible child elements.

✅ Strengths

1. Correct Logic Implementation
The fix properly addresses the root cause by checking if the click target is a child element:

const isClickingKeyboardAccessibleChild =
    target !== element &&
    isElementKeyboardAccessible(target)

This allows motion.button with drag to be dragged while still preventing drag initiation when clicking child buttons/inputs.

2. Excellent Test Coverage
The test suite is comprehensive and follows the project's testing conventions:

• ✅ Tests multiple keyboard-accessible element types (button, input, a)
• ✅ Tests both positive cases (drag should work) and negative cases (drag should be blocked)
• ✅ Properly uses nextFrame() for async frame waiting
• ✅ Follows existing test patterns with MockDrag, motionValue, and the drag() helper
• ✅ Uses proper data-testid attributes for element selection

3. Clear Documentation
The inline comment clearly explains the behavior change and provides a concrete example (motion.button).

🔍 Observations

No Issues Found

• Code quality: Excellent
• Logic: Sound and minimal
• Performance: No concerns (simple comparison operations)
• Security: No concerns
• Style: Follows project conventions (arrow callbacks, const/let usage)

📊 Test Verification

The tests cover all critical scenarios:

1. ✅ Direct click on keyboard-accessible draggable element → drag starts
2. ✅ Click on keyboard-accessible child → drag blocked
3. ✅ Multiple element types tested (button, input, a)

Recommendation

✅ APPROVED - This is a clean, well-tested fix that correctly addresses issue #3473. The implementation is minimal, the logic is sound, and the test coverage is comprehensive. Ready to merge.

Code Review

Summary

This PR fixes a regression where keyboard-accessible elements (button, input, select, textarea, a) with the drag prop couldn't be dragged. The fix correctly distinguishes between clicking on the draggable element itself vs. clicking on a keyboard-accessible child element.

✅ Strengths

1. Correct Logic Fix
The logic change is spot-on. The new condition target !== element && isElementKeyboardAccessible(target) properly implements the intended behavior:

• Before: Blocked drag on ANY keyboard-accessible element (even the draggable itself)
• After: Only blocks drag when clicking on a keyboard-accessible CHILD element

2. Excellent Test Coverage
The test suite is comprehensive and follows the repository's testing guidelines:

• Tests all keyboard-accessible element types: motion.button, motion.input, motion.a
• Tests the blocking behavior for child elements: child-button, child-input
• Uses proper async patterns with nextFrame() as recommended in CLAUDE.md
• Validates both the onDragStart callback and actual position changes (x.get())

3. Clear Documentation
The inline comment clearly explains the rationale and provides a concrete example (motion.button).

🔍 Code Quality

Variable Naming: isClickingKeyboardAccessibleChild is descriptive and self-documenting ✓

Performance: Extracting event.target to a variable avoids repeated casting ✓

Readability: The boolean expression is clear and the negation pattern (!isClickingKeyboardAccessibleChild) matches the existing style ✓

💡 Minor Observations

1. Test Pattern Consistency
All tests follow the same structure which is great for maintainability. Consider if testing motion.select and motion.textarea would add value (though the current coverage already validates the logic comprehensively).

2. Edge Case: Nested Keyboard-Accessible Elements
The fix handles the common case, but consider this scenario:

<motion.button drag>
  <span>
    <a href="#">Link inside button</a>
  </span>
</motion.button>

Clicking the <a> would trigger target !== element (true) and isElementKeyboardAccessible(target) (true), correctly blocking drag. ✓ This works as expected.

3. Event Bubbling
Since the check happens on the pointerdown event of the draggable element, and we're comparing event.target, this correctly identifies where the click originated. The element reference is the draggable element itself. ✓ Logic is sound.

🎯 Testing Validation

To verify the fix works as intended, you could run:

cd packages/framer-motion && yarn test-client --testNamePattern="keyboard accessible elements"

✨ Verdict

LGTM! 🚀

This is a well-implemented fix that:

• Solves the reported issue ([BUG] Cannot drag motion.button with drag prop #3473) correctly
• Maintains the intended behavior from Fix drag gesture triggering on interactive elements inside draggables #3448 (blocking child interactive elements)
• Includes comprehensive test coverage
• Follows the repository's code style and conventions
• Has clear documentation

No blocking issues found. The code is ready to merge.